Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Documentation section to style guide #1062

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

j9liu
Copy link
Contributor

@j9liu j9liu commented Jan 15, 2025

I was writing this as part of CesiumGS/cesium-unreal#1541, but since we're using Doxygen for both Native and Unreal, I figured I should move the section here.

This encapsulates what we do for public API documentation, just formally writing it down for the record.

Copy link
Contributor

@azrogers azrogers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just one small comment.

doc/topics/style-guide.md Outdated Show resolved Hide resolved
Copy link
Contributor

@lilleyse lilleyse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this @j9liu!

doc/topics/style-guide.md Outdated Show resolved Hide resolved

- Put comments **above** the thing being documented (instead of inline).
- Use the `/** ... */` comment style (instead of `///`).
- Use `\ref` when referencing other classes, functions, etc. in the Cesium Native API.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would @link be a better choice? We seem to use both interchangeably and @link is more consistent with JSDoc.

Copy link
Contributor Author

@j9liu j9liu Jan 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We initially were using @link, but I think it wasn't actually working when we generated documentation? 😦 hence #800

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically @ref and @link aren't interchangeable. \ref can have an optional link text specified after the ref name, like @ref CesiumUtility::Uri "URI parsing". Whereas @link is supposed to be paired with @endlink, like @link CesiumUtility::Uri URI parsing @endlink. Because we use the JSDoc inline link tag, {@link name}, Doxygen somehow seems to understand this and doesn't require us to use @endlink. But I'd prefer to use the suggested Doxygen syntax rather than this JSDoc compatibility feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@azrogers do you know if @ref works the same as \ref? If so, I think we should stick with @ref so it matches the rest of the tags we use

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@azrogers @j9liu thanks and good to know, I'll stop using @link and use @ref instead

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@j9liu @ref and \ref work the same. The @ and \ are interchangeable prefixes (I accidentally used \ref where I meant to use @ref in that comment). Personally, I like to use @ in the directives at the start of the line and \ for the directives in the middle of the line (like @brief Some text with a \ref Item tag) to distinguish between inline and non-inline tags. But that's just me and it's probably best if we keep it consistent instead.

doc/topics/style-guide.md Outdated Show resolved Hide resolved
doc/topics/style-guide.md Outdated Show resolved Hide resolved
- `@details`: Use for lengthier explanations, after `@brief`.
- `@tparam`: Describe template parameters.
- `@param`: Denote and describe function parameters.
- `@returns`: Describe the return value of a non-`void` function.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sometimes see @return (singular). I like the plural version more.

@j9liu
Copy link
Contributor Author

j9liu commented Jan 15, 2025

Thanks for the feedback @azrogers and @lilleyse ! I think I addressed all your comments, let me know if you catch anything else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants